Skip to content

feat: watch AuthenticationConfig CM changes#46

Open
Zaggy21 wants to merge 22 commits into
mainfrom
feat/watch-authenticationconfig-cm-changes
Open

feat: watch AuthenticationConfig CM changes#46
Zaggy21 wants to merge 22 commits into
mainfrom
feat/watch-authenticationconfig-cm-changes

Conversation

@Zaggy21
Copy link
Copy Markdown
Contributor

@Zaggy21 Zaggy21 commented Apr 24, 2026

This PR adds auth ConfigMap awareness and moves the watch responsibility to the CareInstruction controller so the ShootController only watches resources on the Garden cluster.

1. Auth CM labeling on first interaction

When authenticationConfigMapName is configured, the shoot controller labels the referenced Greenhouse ConfigMap with:

  • shoot-grafter.cloudoperators.dev/auth-configmap: "true" — marks it as an auth CM
  • shoot-grafter.cloudoperators.dev/careinstruction: <ci-name> — associates it with the owning CareInstruction

2. Watch-triggered reconciliation (moved to CareInstruction controller)

The CareInstruction controller now watches labeled auth ConfigMaps on the Greenhouse cluster. When an auth CM's data changes, the owning CareInstruction is re-enqueued, which detects the data change and restarts the ShootController with the updated config. This keeps the Garden-side OIDC configuration in sync without requiring the ShootController to watch resources on two clusters.

3. In-memory auth CM data

The auth ConfigMap data is passed in-memory from the CareInstruction controller to the ShootController via AuthConfigMapData. The ShootController no longer fetches the auth CM from the Greenhouse cluster directly.

@Zaggy21 Zaggy21 requested a review from a team as a code owner April 24, 2026 12:30
@Zaggy21 Zaggy21 linked an issue Apr 24, 2026 that may be closed by this pull request
4 tasks
@Zaggy21 Zaggy21 requested a review from Copilot April 27, 2026 08:40
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds cross-cluster reconciliation for Shoot OIDC configuration by labeling referenced Greenhouse AuthenticationConfiguration ConfigMaps and watching them for changes, so matching Shoots are re-enqueued when the auth ConfigMap updates.

Changes:

  • Label Greenhouse auth ConfigMaps on first interaction with AuthConfigMapLabel and CareInstructionLabel.
  • Add a Greenhouse-cache-backed watch to the Shoot controller to re-enqueue matching Shoots on auth ConfigMap changes.
  • Extend tests and update README documentation for the new labeling + watch behavior.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
controller/shoot/shoot_controller.go Adds Greenhouse ConfigMap watch and re-enqueue mapping for Shoots.
controller/shoot/auth.go Adds CareInstruction ownership labeling to the referenced Greenhouse auth ConfigMap.
controller/careinstruction/careinstruction_controller.go Passes the Greenhouse manager into ShootController so it can watch Greenhouse resources.
controller/shoot/shoot_controller_test.go Starts the Greenhouse manager in tests and adds watch-triggered reconciliation coverage.
README.md Documents auth ConfigMap labeling + watch behavior and updates related references.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread controller/shoot/shoot_controller_test.go
Comment thread README.md Outdated
Comment thread controller/shoot/auth.go Outdated
Comment thread controller/shoot/auth.go Outdated
Comment thread controller/shoot/shoot_controller.go Outdated
Comment thread controller/shoot/shoot_controller_test.go Outdated
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds cross-cluster watch support so changes to a labeled Greenhouse AuthenticationConfiguration ConfigMap trigger immediate re-reconciliation of affected Shoots, keeping Garden-side OIDC configuration up to date. It also ensures the referenced auth ConfigMap is labeled on first interaction to make the watch predicate work.

Changes:

  • Label Greenhouse auth ConfigMaps with authconfigmap=true and the owning careinstruction=<name> on first use (via merge-patch).
  • Add a Greenhouse-cache-backed watch in the Shoot controller to re-enqueue matching Shoots on auth ConfigMap changes.
  • Extend controller tests and update README documentation for the labeling/watch behavior.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
controller/shoot/shoot_controller.go Adds Greenhouse ConfigMap watch + enqueue mapping for watch-triggered reconciles.
controller/shoot/auth.go Switches to merge-patching labels and adds CareInstruction ownership label logic.
controller/careinstruction/careinstruction_controller.go Passes Greenhouse manager into ShootController to enable cross-cluster watches.
controller/shoot/shoot_controller_test.go Starts Greenhouse manager/cache for tests and adds watch-triggered reconciliation coverage.
api/v1alpha1/careinstruction_types.go Unifies auth ConfigMap label key under shoot-grafter.cloudoperators.dev.
README.md Documents labeling + watch behavior and updates OIDC doc link.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread controller/shoot/shoot_controller.go Outdated
Comment thread README.md Outdated
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves OIDC AuthenticationConfiguration synchronization by labeling the referenced Greenhouse auth ConfigMap on first interaction and adding a Greenhouse-cache-backed watch that re-enqueues matching Shoots when that ConfigMap changes. It also standardizes the auth ConfigMap label key under the shoot-grafter.cloudoperators.dev prefix and updates docs/tests accordingly.

Changes:

  • Add labeling of Greenhouse AuthenticationConfiguration ConfigMaps with auth-configmap=true and the owning careinstruction label (using a merge patch).
  • Add a Greenhouse ConfigMap watch (via Greenhouse manager cache) to trigger immediate re-reconciliation of matching Shoots on auth CM updates.
  • Update label key constant + README and extend tests to cover labeling and watch-triggered reconciliation.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
controller/shoot/shoot_controller.go Adds cross-cluster ConfigMap watch and re-enqueue mapping for matching Shoots.
controller/shoot/auth.go Switches to merge-patching labels and adds CareInstruction ownership labeling with conflict handling.
controller/shoot/shoot_controller_test.go Starts/syncs Greenhouse manager cache and adds tests for labeling + watch-triggered reconciliation.
controller/careinstruction/careinstruction_controller.go Wires Greenhouse manager into ShootController for cross-cluster watches.
api/v1alpha1/careinstruction_types.go Updates the auth ConfigMap label key constant to the new .dev/auth-configmap value.
README.md Documents new labeling/watch behavior and updates OIDC doc link + field description.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread controller/shoot/shoot_controller.go Outdated
Comment thread controller/shoot/shoot_controller_test.go Outdated
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR extends the Shoot controller to keep Garden-side OIDC authentication configuration in sync with a referenced Greenhouse AuthenticationConfiguration ConfigMap by (1) labeling the ConfigMap on first interaction and (2) reconciling Shoots when that labeled ConfigMap changes.

Changes:

  • Add labeling of the referenced Greenhouse auth ConfigMap with both an “auth CM” label and an owning CareInstruction label.
  • Add a cross-cluster watch (via the Greenhouse manager cache) that re-enqueues all matching Shoots when the labeled auth ConfigMap changes.
  • Update the auth ConfigMap label key constant, tests, and README documentation accordingly.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
controller/shoot/shoot_controller.go Adds Greenhouse cache-backed ConfigMap watch and re-enqueue mapping for auth ConfigMap changes.
controller/shoot/auth.go Patches Greenhouse auth ConfigMap labels (auth + careinstruction) without overwriting data.
controller/careinstruction/careinstruction_controller.go Passes the Greenhouse manager into the ShootController for cross-cluster watches.
api/v1alpha1/careinstruction_types.go Renames the auth ConfigMap label key to the .dev/auth-configmap form.
controller/shoot/shoot_controller_test.go Starts/syncs the Greenhouse manager cache in tests and adds watch-triggered reconciliation coverage.
README.md Documents labeling + watch behavior and updates the referenced Greenhouse docs link.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread controller/shoot/shoot_controller.go Outdated
// GreenhouseMgr is passed so the ShootController can watch Greenhouse cluster resources (e.g. auth CMs).
sc := &shoot.ShootController{
GreenhouseClient: r.Client,
GreenhouseMgr: r.Manager,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not super convinced that watching resources on two different Clusters is the way to go here.
I would opt to actually watch the CM changes in the CareInstruction controller for separation of concerns.
That would involve:

Looping in @abhijith-darshan for his opinion

@Zaggy21 Zaggy21 force-pushed the feat/watch-authenticationconfig-cm-changes branch 2 times, most recently from 7d85659 to 4193706 Compare May 12, 2026 08:51
Zaggy21 and others added 13 commits May 12, 2026 10:52
…enhouse auth CM changes

On-behalf-of: @SAP krzysztof.zagorski@sap.com
Signed-off-by: Zaggy21 <k.zaggy@gmail.com>
…iggered reconciliation

On-behalf-of: @SAP krzysztof.zagorski@sap.com
Signed-off-by: Zaggy21 <k.zaggy@gmail.com>
On-behalf-of: @SAP krzysztof.zagorski@sap.com
Signed-off-by: Zaggy21 <k.zaggy@gmail.com>
* feat: trigger Shoot reconciliation on OIDC ConfigMap updates

On-behalf-of: @SAP krzysztof.zagorski@sap.com

* simplify the conditions for triggering shoot reconciliation

On-behalf-of: @SAP krzysztof.zagorski@sap.com

* update docs

On-behalf-of: @SAP krzysztof.zagorski@sap.com

* add bin to gitignore, remove invalid G118 from gosec excludes, regenerate files

On-behalf-of: @SAP krzysztof.zagorski@sap.com

* revert golangci config due to mismatch between CI and local version

On-behalf-of: @SAP krzysztof.zagorski@sap.com

* update go to 1.26.0

On-behalf-of: @SAP krzysztof.zagorski@sap.com

* update project with go-makefile-maker

On-behalf-of: @SAP krzysztof.zagorski@sap.com
Signed-off-by: Zaggy21 <k.zaggy@gmail.com>
* chore: bump Go base image to 1.26.0 to match go.mod

On-behalf-of: @SAP <mikolaj.kucinski@sap.com>

* chore: make typos binary cleanup idempotent

On-behalf-of: @SAP <mikolaj.kucinski@sap.com>
Signed-off-by: Zaggy21 <k.zaggy@gmail.com>
On-behalf-of: @SAP krzysztof.zagorski@sap.com
Signed-off-by: Zaggy21 <k.zaggy@gmail.com>
On-behalf-of: @SAP krzysztof.zagorski@sap.com
Signed-off-by: Zaggy21 <k.zaggy@gmail.com>
On-behalf-of: @SAP krzysztof.zagorski@sap.com
Signed-off-by: Zaggy21 <k.zaggy@gmail.com>
On-behalf-of: @SAP krzysztof.zagorski@sap.com
Signed-off-by: Zaggy21 <k.zaggy@gmail.com>
On-behalf-of: @SAP krzysztof.zagorski@sap.com
Signed-off-by: Zaggy21 <k.zaggy@gmail.com>
On-behalf-of: @SAP krzysztof.zagorski@sap.com
Signed-off-by: Zaggy21 <k.zaggy@gmail.com>
…auth-configmap

On-behalf-of: @SAP krzysztof.zagorski@sap.com
Signed-off-by: Zaggy21 <k.zaggy@gmail.com>
…nager context in tests

On-behalf-of: @SAP krzysztof.zagorski@sap.com
Signed-off-by: Zaggy21 <k.zaggy@gmail.com>
Zaggy21 added 3 commits May 12, 2026 10:52
On-behalf-of: @SAP krzysztof.zagorski@sap.com
Signed-off-by: Zaggy21 <k.zaggy@gmail.com>
On-behalf-of: @SAP krzysztof.zagorski@sap.com
Signed-off-by: Zaggy21 <k.zaggy@gmail.com>
…on controller, adapt tests

On-behalf-of: @SAP krzysztof.zagorski@sap.com
Signed-off-by: Zaggy21 <k.zaggy@gmail.com>
@Zaggy21 Zaggy21 force-pushed the feat/watch-authenticationconfig-cm-changes branch from 4193706 to 1807100 Compare May 12, 2026 08:52
Zaggy21 added 5 commits May 12, 2026 11:13
On-behalf-of: @SAP krzysztof.zagorski@sap.com
Signed-off-by: Zaggy21 <k.zaggy@gmail.com>
On-behalf-of: @SAP krzysztof.zagorski@sap.com
Signed-off-by: Zaggy21 <k.zaggy@gmail.com>
On-behalf-of: @SAP krzysztof.zagorski@sap.com
Signed-off-by: Zaggy21 <k.zaggy@gmail.com>
On-behalf-of: @SAP krzysztof.zagorski@sap.com
Signed-off-by: Zaggy21 <k.zaggy@gmail.com>
On-behalf-of: @SAP krzysztof.zagorski@sap.com
Signed-off-by: Zaggy21 <k.zaggy@gmail.com>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 13 out of 15 changed files in this pull request and generated 5 comments.

Comment thread README.md Outdated
Comment thread controller/shoot/auth.go Outdated
Comment thread controller/shoot/auth.go
Comment thread controller/careinstruction/careinstruction_controller.go
Comment thread controller/careinstruction/careinstruction_controller.go
…e events, fix fetchAuthConfigMapData NotFound contract

On-behalf-of: @SAP krzysztof.zagorski@sap.com
Signed-off-by: Zaggy21 <k.zaggy@gmail.com>
@github-actions
Copy link
Copy Markdown

Merging this branch will not change overall coverage

Impacted Packages Coverage Δ 🤖
github.com/cloudoperators/shoot-grafter/api/v1alpha1 0.00% (ø)
github.com/cloudoperators/shoot-grafter/controller/careinstruction 0.00% (ø)
github.com/cloudoperators/shoot-grafter/controller/shoot 0.00% (ø)

Coverage by file

Changed files (no unit tests)

Changed File Coverage Δ Total Covered Missed 🤖
github.com/cloudoperators/shoot-grafter/api/v1alpha1/careinstruction_types.go 0.00% (ø) 0 0 0
github.com/cloudoperators/shoot-grafter/controller/careinstruction/careinstruction_controller.go 0.00% (ø) 0 0 0
github.com/cloudoperators/shoot-grafter/controller/careinstruction/metrics.go 0.00% (ø) 0 0 0
github.com/cloudoperators/shoot-grafter/controller/shoot/auth.go 0.00% (ø) 0 0 0
github.com/cloudoperators/shoot-grafter/controller/shoot/shoot_controller.go 0.00% (ø) 0 0 0

Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code.

Changed unit test files

  • github.com/cloudoperators/shoot-grafter/controller/shoot/auth_test.go
  • github.com/cloudoperators/shoot-grafter/controller/shoot/shoot_controller_test.go

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 14 out of 16 changed files in this pull request and generated 5 comments.

Comment thread controller/shoot/auth.go
Comment on lines +33 to +36
if r.AuthConfigMapData == nil || r.AuthConfigMapData[authConfigKey] == "" {
return fmt.Errorf("AuthenticationConfiguration ConfigMap %s does not contain config.yaml (data not available)",
r.CareInstruction.Spec.AuthenticationConfigMapName)
}
// Fetch the current auth ConfigMap data so we can detect changes and pass it to the ShootController.
currentAuthCMData, authCMErr := r.fetchAuthConfigMapData(ctx, &careInstruction)
if authCMErr != nil {
r.Info("failed to fetch auth ConfigMap data, will proceed without it", "error", authCMErr)
Comment on lines 35 to 40
// CareInstructionLabel is the label used to identify resources created by this CareInstruction.
CareInstructionLabel = "shoot-grafter.cloudoperators.dev/careinstruction"

// AuthConfigMapLabel is the label used to identify AuthenticationConfiguration ConfigMaps
AuthConfigMapLabel = "shoot-grafter.cloudoperators/authconfigmap"
AuthConfigMapLabel = "shoot-grafter.cloudoperators.dev/auth-configmap"

Comment on lines +91 to +99
// Watch auth ConfigMaps on the Greenhouse cluster. When their Data changes, re-enqueue the owning
// CareInstruction so reconcileManager detects the change and restarts the ShootController with
// the updated CM data.
Watches(&corev1.ConfigMap{}, handler.EnqueueRequestsFromMapFunc(r.enqueueCareInstructionForAuthConfigMap),
builder.WithPredicates(
clientutil.PredicateHasLabel(v1alpha1.AuthConfigMapLabel),
authCMDataChangedPredicate,
),
).
Comment on lines 2052 to 2056
It("should annotate Shoot with gardener.cloud/operation=reconcile when ConfigMap content changes", func() {
// Create Greenhouse auth ConfigMap
greenhouseAuthCM := &corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Name: "greenhouse-oidc-config",
Namespace: "default",
},
Data: map[string]string{
"config.yaml": `apiVersion: apiserver.config.k8s.io/v1beta1
kind: AuthenticationConfiguration
jwt:
- issuer:
url: https://greenhouse.example.com
audiences:
- greenhouse
claimMappings:
username:
claim: sub
prefix: 'greenhouse:'
`,
},
}
Expect(test.K8sClient.Create(test.Ctx, greenhouseAuthCM)).To(Succeed(), "should create Greenhouse auth ConfigMap")
// Note: the Greenhouse auth ConfigMap was created in BeforeEach.

// Create a shoot that ALREADY has the OIDC ConfigMap reference
shoot := &gardenerv1beta1.Shoot{
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FEAT shoot-grafter] - Watch AuthenticationConfig CM changes

4 participants